feat: Add async FDv1 polling data source and feature requester - #475
feat: Add async FDv1 polling data source and feature requester#475jsonbailey wants to merge 4 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| self._cache[uri] = CacheEntry(data=data, etag=etag) | ||
| log.debug("%s response status:[%d] From cache? [%s] ETag:[%s]", uri, r.status, from_cache, etag) | ||
|
|
||
| return {FEATURES: data['flags'], SEGMENTS: data['segments']} |
There was a problem hiding this comment.
304 skips re-init never triggers
Medium Severity
AsyncFeatureRequesterImpl returns cached payload data on HTTP 304, but AsyncPollingUpdateProcessor treats None as “not modified” and only then skips init. The skip path never runs with the real requester, so every unchanged poll re-inits the store (and the update sink’s change-detection path). The polling comment and 304 unit test both assume a None return for not-modified responses.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b8b7f52. Configure here.
Drop the async feature requester's duplicate endpoint definition; use the shared constant from datasource_common instead.
b8b7f52 to
16c4438
Compare
- Don't set _ready on a generic poll exception, so a transient error during startup no longer ends start_wait early (matches sync). - Close the owned HTTP transport on stop: the feature requester tracks whether it created the transport and exposes close(); the polling processor awaits it. - Drop the dead 'all_data is not None' guard (the requester returns cached data on 304, never None) and the fictional None-return polling test.
AsyncRepeatingTask gains wait_stopped() to await the cancelled task; the polling processor's stop() now waits for the in-flight poll to unwind before closing the requester's transport, so awaiting stop() guarantees background work has stopped and the transport isn't closed under a live request.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fff0f5e. Configure here.
| try: | ||
| await task | ||
| except asyncio.CancelledError: | ||
| pass |
There was a problem hiding this comment.
Stop wait swallows cancellation
Medium Severity
wait_stopped awaits the worker with await task and catches all CancelledErrors. That conflates the expected cancellation of the worker (after stop()) with cancellation of the caller itself, so a timed or cancelled stop() can swallow the cancel, continue into close(), and return successfully. Nearby helpers like join_handle use asyncio.wait to avoid this.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit fff0f5e. Configure here.
1 participant
Overview
PR 7 of the SDK-60 async epic: the async FDv1 polling data source and feature requester.
async_polling.py— async FDv1 polling update processor. Polls the feature requester on an interval and pushes flag/segment data into the data source update sink, updating data source status (VALID / OFF) as appropriate.async_feature_requester.py— async FDv1 feature requester that fetches the full flag/segment payload over HTTP.test_async_polling.py— unit tests for the async polling update processor.Stacking
This PR is stacked on #464 (base branch
jb/sdk-2743/async-fdv1-streaming), which provides the shareddatasource_commonmodule these files import. Until #464 merges, this PR will also show #464's commits in its diff; a rebase after #464 merges will drop them, leaving only the three files here.SDK-2825
Note
Medium Risk
New async data-source path affects how flag data is fetched and shut down; behavior is well tested but ties into client initialization and HTTP lifecycle.
Overview
Adds the async FDv1 polling path so the async client can load flags and segments on a poll interval instead of streaming.
AsyncFeatureRequesterImplperforms GETs to the FDv1 polling endpoint viaAsyncHTTPTransport, with gzip, optional payload filter query params, and ETag / 304 caching. It only closes the transport when it created it.AsyncPollingUpdateProcessorruns periodic fetches throughAsyncRepeatingTask, initializes the store viasink_or_store, sets ready when the store is populated, and drives data source status (VALID,INTERRUPTED,OFF) for recoverable vs unrecoverable HTTP errors and unknown exceptions—aligned with sync polling behavior (e.g. transient errors keep polling without releasing ready).AsyncRepeatingTask.wait_stopped()letsstop()wait for the background task to unwind after cancel, whichAsyncPollingUpdateProcessor.stop()uses so the HTTP transport is not closed mid-request.Unit tests cover requester caching/ownership, processor init/error/stop ordering, and sink updates.
Reviewed by Cursor Bugbot for commit fff0f5e. Bugbot is set up for automated code reviews on this repo. Configure here.